Skip to content

Conversation

@jamesagnew
Copy link

JSON doesn't allow a leading decimal in a float (e.g. { "someval": .123 } ) and Jackson rightly errors out when you try to parse this.

I have a need however (mostly caused by a previous Gson implementation now replaced with Jackson in HAPI FHIR silently accepting this syntax) to parse JSON containing this invalid format.

The attached pull request adds a read feature enabling the parsing of these leading decimal points.

@cowtowncoder
Copy link
Member

@jamesagnew I thought I added a comment here but looks like I did not. So, I think this makes sense: I'll have a look and may be able to merge this to be included in 2.11 (will cherry-pick from master commit, should be fine).
Thank you for submitting the patch.

But one thing I would need (if I haven't yet asked for it; I don't think I have) before merging the patch is CLA. It's found from

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and is usually easiest to print, fill & sign, scan/photo, email to info at fasterxml dot com.
This is only needed once before the first contribution, one is fine for any and all submissions.

Thank you once again!

@jamesagnew
Copy link
Author

Thanks @cowtowncoder! CLA has been signed and submitted.

cowtowncoder added a commit that referenced this pull request Apr 21, 2020
@cowtowncoder
Copy link
Member

Quick note: with CLA, I am proceeding this one -- but merging pieces manually, mostly due to challenges b/w master and 2.11 branches (wrt refactoring of JsonReadFeature / JsonParser.Feature). Should be done soon.

cowtowncoder added a commit that referenced this pull request Apr 23, 2020
cowtowncoder added a commit that referenced this pull request Apr 23, 2020
cowtowncoder added a commit that referenced this pull request Apr 23, 2020
cowtowncoder added a commit that referenced this pull request Apr 23, 2020
@cowtowncoder cowtowncoder modified the milestones: 2.11., 2.11.0 Apr 23, 2020
@cowtowncoder
Copy link
Member

Finally got it all done; added async-parser tests too (those are separate as input source is different). Thank you again for contributing this! Hoping to now release 2.11.0 by the end of the week.

@jamesagnew
Copy link
Author

@cowtowncoder Appreciate the fast response to a random PR from a new contributor! You're better at this than I am. :)

@cowtowncoder
Copy link
Member

@jamesagnew I've had some experience. But the patch was good and made this pretty easy. Adding new support for non-standard parsing is often tricky due to my over-optimization coding habits, but this turned out to be easier than I'd have expected. :)

Thank you once again for contributing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants